Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(archive): add waku archive implementation #1406

Merged
merged 1 commit into from
Nov 22, 2022
Merged

feat(archive): add waku archive implementation #1406

merged 1 commit into from
Nov 22, 2022

Conversation

LNSD
Copy link
Contributor

@LNSD LNSD commented Nov 21, 2022 •
edited
Loading

The current PR adds the new Waku archive to the codebase. The code is not used yet. Some work replacing the current MessageStore code is needed. This PR got precipitated due to #1400.

  • Split the message storage logic out of the Waku store protocol module.
  • Decouple the message validation logic.
    • Extract the current message validation logic into the "default" message validator.
    • Improve the "invalid message" metrics by differentiating invalid message types.
  • Collocate under the waku_archive module the "driver" (formerly message_store) and "retention_policy" modules.
  • Ported and added new test cases covering the functionality.

ℹ️ Instead of performing the fixes and improvements for #1400 in the message_store code, they will be performed in the waku_archive module (e.g., the sqlite_driver module).

@LNSD LNSD self-assigned this Nov 21, 2022
@status-im-auto
Copy link
Collaborator

status-im-auto commented Nov 21, 2022 •
edited
Loading

Jenkins Builds

❔ Commit #️⃣ Finished (UTC) Duration Platform Result
❌ ad19781 #1 2022-11-21 11:43:26 ~14 min linux 📄log
❌ ad19781 #1 2022-11-21 11:43:35 ~14 min macos 📄log
✔️ 9f10e1c #2 2022-11-22 12:39:42 ~16 min macos 📦bin
✔️ 9f10e1c #2 2022-11-22 12:41:34 ~17 min linux 📦bin

jm-clius
jm-clius approved these changes Nov 22, 2022
View reviewed changes
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Quite a lot. :) I went through everything in as much detail as I could. I can see all of the functionality of the "old" store implementations and the archive/driver interfaces make sense to me. Presumably adding e.g. PostgreSQL should (after integrating this) be as simple as writing a new driver.

@LNSD LNSD force-pushed the feat-waku-archive branch from ad19781 to 9f10e1c Compare November 22, 2022 12:23
@LNSD
Copy link
Contributor Author

LNSD commented Nov 22, 2022 •
edited
Loading

Regarding #1400, some changes have been performed in the SQLite driver queries:

While extending the query's test coverage, an inconsistency has been detected in the SQLite query used when providing a cursor and a pubsub topic. Ignoring the pubsub value contained in the cursor seems to fix the issue.

This was referenced Nov 22, 2022
Closed
Merged
@LNSD
Copy link
Contributor Author

LNSD commented Nov 22, 2022

As these changes precipitated, there are some sharp edges and minor details that need extra work. That work will be held in future PRs.

Merging this PR.

@LNSD LNSD merged commit 0188ec1 into master Nov 22, 2022
@LNSD LNSD deleted the feat-waku-archive branch November 22, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jm-clius jm-clius jm-clius approved these changes

@alrevuelta alrevuelta Awaiting requested review from alrevuelta

@rymnc rymnc Awaiting requested review from rymnc

Assignees

@LNSD LNSD

Labels
None yet
Projects
Waku
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LNSD @status-im-auto @jm-clius